all_channels: Replace three dot menu with long press gesture and add on tap to feed #1915

chungwwei · 2025-10-11T09:10:19Z

Fixes #1914

Before and After still

Page View

Before	After

After Max Sys Font

After Max Sys Font

Long Press View

Long Press on Public Channel	~~Long Press on Private Channel~~ Not part of this PR anymore

Gesture Press Views

Gesture press in default sys font

screen-20251028-1754402.mp4

Unsub from private channel 200 milliseconds fade. Not part of PR

screen-20251028-1749162.mp4

gnprice

Thanks @chungwwei for writing this PR! Generally this looks good. Various small comments below.

lib/widgets/all_channels.dart

test/widgets/all_channels_test.dart

gnprice · 2025-10-15T23:36:41Z

The screencast video is helpful, thanks.

Please also include a still screenshot. See discussion in our instructions:
https://zulip.readthedocs.io/en/latest/contributing/reviewable-prs.html#demonstrating-visual-changes

The interactions in the screencast are somewhat hard to follow, because the viewer can't see where on the screen you were tapping. When you take a screen recording, there's an option to have it show touches on the screen (here's Android docs); you should basically always enable that option when making a screencast to demonstrate some UI behavior.

gnprice · 2025-10-17T00:34:41Z

Thanks for the revision. One reply in a subthread above: #1915 (comment)

Please also add a screenshot, and a screen recording with touches visible, so that we can review the visual changes. See details in my comment above: #1915 (comment)

gnprice · 2025-10-17T00:37:00Z

Oh and please see our Git style guide:
https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html#each-commit-must-be-coherent

In this PR, the two commits should be squashed together, rather than have one commit for the first revision and then another commit that adjusts those changes.

gnprice · 2025-10-18T02:46:03Z

Thanks for the revision.

Please see our Git style guide (linked above and in our README) for how to write clear commit messages. See also the reply above at #1915 (comment) .

In the screenshots, the toggles are much too close to the right edge of the screen. (Notice that they're visually much closer than the content at the left is to the left edge, and much closer than the three-dots icon is in main.) They should get appropriate padding.

chungwwei · 2025-10-18T14:09:39Z

Thanks Greg for the review! Hopefully the commits are following the style guide now.

I believe that GestureDetector was probably not the best fit to use in the PR. Other parts of the app like the Channel Page uses InkWell, which has the rippling effect when long press.

gnprice

Thanks for the revision, and the updated screenshots. Some comments below.

The commit messages are mostly good now, but the body should get line-wrapped. See this section:
https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html#formatting-guidelines

To demonstrate why, here's how the first commit message looks when I'm reviewing the changes (with git log --stat -p):

lib/widgets/all_channels.dart

gnprice

Thanks for the revision!

lib/widgets/all_channels.dart

gnprice

Thanks for all your work on this PR! I think this is very close now.

lib/widgets/all_channels.dart

gnprice · 2025-10-31T00:36:29Z

lib/widgets/all_channels.dart

+        MessageListPage.buildRoute(context: context,
+          narrow: ChannelNarrow(channel.streamId))),
+      onLongPress: () => showChannelActionSheet(context, channelId: channel.streamId),
+      child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 44),


commit-sequence nit:

all_channels: Make row item at least 44px tall with ConstrainedBox Make sure every row is at least 44px in height by specifying BoxConstraints(minHeight: 44) to meet touch target requirement.

Did the removal of the three-dots button make these rows shorter than they had been?

If so, then this commit adding the 44px min-height should go before the commit that removes the three-dots button. That way we don't have an intermediate state where the UI is less good in some ways than it was before.

Yeah, reordering the commits.
Private channel row with no content access is shorter (20px high) without the three-dots button.

gnprice · 2025-10-31T00:47:39Z

lib/widgets/all_channels.dart

+            IgnorePointer(ignoring: !hasContentAccess,
+              child: AnimatedOpacity(opacity: !hasContentAccess ? 0 : 1,
+                duration: Duration(milliseconds: 200),
+                child: _SubscribeToggle(channel: channel)))


I'm not convinced we need this change, i.e. the last commit:
5a60145 all_channels: Make toggle transparent and non-interactive when noContentAccess

It's not a super common interaction in the first place (though it is an important one) to subscribe and unsubscribe to channels; and I think it's going to be a lot less common than that to unsubscribe when that means you lose access to the channel. So if that sometimes means that the row gets a bit shorter and the rows below jump up a bit, I think that's a pretty minor issue.

(It's also not clear to me whether that ever actually does happen, or if it's a theoretical issue.)

Conversely, having this here makes the code somewhat more complicated to understand. It also adds a nontrivial widget (the toggle) to the tree in cases where the user won't actually see it; and does so for each element of a potentially long list, so it has a potential performance cost.

Went down the rabbit hole on this one, but learned a few things along the way, like the 44px touch targets and some git rebase tips.

Dropped the commit and updated the statics screenshots.

gnprice · 2025-11-01T00:07:57Z

Thanks! This all looks good now — merging.

Make sure every row is at least 44px in height by specifying BoxConstraints(minHeight: 44) to meet touch target requirement.

Make the All Channels Page consistent with the Channel Page, as navigating to channel feed and opening the bottom sheet currently uses the three-dot menu icon, which differs from the gesture pattern used in the Channel Page. Fixes part of zulip#1914

Previously, long name could span multiple lines when the system font size was large, causing the row height to become insconsistent with other items. Limiting the name to a single line ensures consistent row height, similar to how it's done on the Channel Page. Fixes zulip#1914

gnprice added maintainer review PR ready for review by Zulip maintainers integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Oct 15, 2025

gnprice reviewed Oct 15, 2025

View reviewed changes

gnprice mentioned this pull request Oct 16, 2025

docs: Request showing touches/clicks in screencast videos zulip/zulip#36287

Merged

chungwwei requested a review from gnprice October 16, 2025 12:01

chungwwei force-pushed the all-channels-ui branch 3 times, most recently from 1a64215 to a3105d0 Compare October 17, 2025 16:58

chungwwei force-pushed the all-channels-ui branch 3 times, most recently from 698f215 to 4ae4515 Compare October 18, 2025 12:24

chungwwei force-pushed the all-channels-ui branch from 508fa04 to 173bf47 Compare October 18, 2025 18:12

gnprice reviewed Oct 21, 2025

View reviewed changes

lib/widgets/all_channels.dart Show resolved Hide resolved

lib/widgets/all_channels.dart Outdated Show resolved Hide resolved

lib/widgets/all_channels.dart Outdated Show resolved Hide resolved

chungwwei force-pushed the all-channels-ui branch 5 times, most recently from 1f3725d to e775639 Compare October 21, 2025 17:12

chungwwei requested a review from gnprice October 21, 2025 17:46

chungwwei force-pushed the all-channels-ui branch 5 times, most recently from 78b7ca3 to 3acba11 Compare October 26, 2025 02:07

chungwwei force-pushed the all-channels-ui branch 2 times, most recently from 7bb92ae to dac2f4b Compare October 26, 2025 17:57

gnprice reviewed Oct 27, 2025

View reviewed changes

lib/widgets/all_channels.dart Outdated Show resolved Hide resolved

lib/widgets/all_channels.dart Outdated Show resolved Hide resolved

chungwwei force-pushed the all-channels-ui branch 4 times, most recently from ca4e951 to 5a60145 Compare October 28, 2025 18:43

chungwwei requested a review from gnprice October 28, 2025 22:04

gnprice reviewed Oct 31, 2025

View reviewed changes

chungwwei force-pushed the all-channels-ui branch 3 times, most recently from f8c4aad to ef41ad2 Compare October 31, 2025 13:43

chungwwei requested a review from gnprice October 31, 2025 14:07

chungwwei added 3 commits October 31, 2025 17:08

all_channels: Make row item at least 44px tall with ConstrainedBox

9043c5f

Make sure every row is at least 44px in height by specifying BoxConstraints(minHeight: 44) to meet touch target requirement.

gnprice force-pushed the all-channels-ui branch from ef41ad2 to 8942c91 Compare November 1, 2025 00:08

gnprice merged commit 8942c91 into zulip:main Nov 1, 2025
1 check passed

all_channels: Replace three dot menu with long press gesture and add on tap to feed #1915

all_channels: Replace three dot menu with long press gesture and add on tap to feed #1915

Conversation

chungwwei commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Page View

Gesture press in default sys font

Unsub from private channel 200 milliseconds fade. Not part of PR

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnprice commented Oct 15, 2025

Uh oh!

gnprice commented Oct 17, 2025

Uh oh!

gnprice commented Oct 17, 2025

Uh oh!

gnprice commented Oct 18, 2025

Uh oh!

chungwwei commented Oct 18, 2025

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gnprice Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

chungwwei Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

gnprice Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

chungwwei Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

gnprice commented Nov 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chungwwei commented Oct 11, 2025 •

edited

Loading